Skip to content

Conversation

@yixinshark
Copy link
Contributor

@yixinshark yixinshark commented Sep 19, 2025

…s not displayed

When you click the app tray button, the hover background is displayed normally

Log: as title
Pms: BUG-288633

Summary by Sourcery

Fix hover background disappearance and implement toggle behavior for stash popup in the application tray button

Bug Fixes:

  • Include stash popup visibility in hover state to retain hover background when popup is open
  • Toggle the stash popup on click instead of always opening it

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 19, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR updates the tray button behavior to maintain its hover background while the stash popup is open and changes the click handler to toggle the popup visibility instead of always opening it.

File-Level Changes

Change Details Files
Maintain hover background when stash popup is visible
  • Include stashedPopup.popupVisible in D.ColorSelector.hovered condition
panels/dock/tray/package/ActionShowStashDelegate.qml
Toggle stash popup on click instead of always opening
  • Replace unconditional stashedPopup.open() with if/else to open or close the popup
  • Ensure toolTip.close() still executes after click
panels/dock/tray/package/ActionShowStashDelegate.qml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, yixinshark

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…s not displayed

When you click the app tray button, the hover background is displayed normally

Log: as title
Pms: BUG-288633
@yixinshark yixinshark force-pushed the fix-applicationTrayHover branch from 21260e1 to 3dc6db7 Compare September 19, 2025 09:14
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码是一个QML文件,用于实现一个系统托盘中的动作按钮。我来审查这段代码的语法逻辑、代码质量、代码性能和代码安全。

语法逻辑

  1. hover状态判断逻辑

    D.ColorSelector.hovered: (isDropHover && DDT.TraySortOrderModel.actionsAlwaysVisible) || hoverHandler.hovered || stashedPopup.popupVisible

    这行代码判断按钮是否应该显示悬停状态,条件包括:

    • 拖放悬停且动作始终可见
    • 鼠标悬停处理器检测到的悬停
    • 弹出窗口可见

    这个逻辑看起来合理,但可能过于复杂,可以考虑将其拆分为更清晰的条件表达式。

  2. 点击事件处理

    onClicked: {
        if (stashedPopup.popupVisible) {
            stashedPopup.close();
        } else {
            stashedPopup.open();
        }
        toolTip.close();
    }

    点击事件处理逻辑正确,实现了切换弹出窗口的显示/隐藏状态,并在点击时关闭工具提示。

代码质量

  1. 代码可读性

    • 代码整体结构清晰,逻辑易于理解。
    • 变量命名规范,如stashedPopuptoolTip等。
    • 缩进和格式规范。
  2. 代码复用

    • 没有看到明显的代码重复问题。
  3. 代码组织

    • 组件结构合理,各个功能模块划分清晰。

代码性能

  1. hover状态判断

    • 当前的悬停状态判断逻辑涉及多个条件,每次状态变化时都会重新计算,但由于条件简单,性能影响可以忽略不计。
  2. 点击事件处理

    • 点击事件处理逻辑简单直接,性能良好。
  3. 属性绑定

    • D.ColorSelector.hovered的绑定可能会频繁触发,因为stashedPopup.popupVisible会变化,但这是必要的,以确保视觉反馈及时更新。

代码安全

  1. 属性访问

    • 代码中访问了DDT.TraySortOrderModel.actionsAlwaysVisible,假设这是一个全局模型,需要确保其存在且可访问,否则可能导致运行时错误。
  2. 事件处理

    • 点击事件处理中没有明显的安全隐患。

改进建议

  1. 简化悬停状态判断逻辑

    D.ColorSelector.hovered: {
        let shouldHover = isDropHover && DDT.TraySortOrderModel.actionsAlwaysVisible;
        shouldHover = shouldHover || hoverHandler.hovered;
        shouldHover = shouldHover || stashedPopup.popupVisible;
        return shouldHover;
    }

    或者提取为一个计算属性:

    readonly property bool isHovered: (isDropHover && DDT.TraySortOrderModel.actionsAlwaysVisible) || hoverHandler.hovered || stashedPopup.popupVisible
    D.ColorSelector.hovered: isHovered
  2. 添加错误处理

    • 在访问DDT.TraySortOrderModel.actionsAlwaysVisible时,可以添加错误处理,防止模型不存在时导致崩溃。
  3. 优化点击事件处理

    • 当前的点击事件处理逻辑已经很好,但可以考虑添加一个切换函数,提高代码可读性:
    function togglePopup() {
        if (stashedPopup.popupVisible) {
            stashedPopup.close();
        } else {
            stashedPopup.open();
        }
        toolTip.close();
    }
    
    onClicked: togglePopup()
  4. 添加注释

    • 对于复杂的逻辑,如悬停状态判断,可以添加注释,解释各个条件的含义。
  5. 考虑使用状态机

    • 对于按钮的状态管理,可以考虑使用QML的状态机,使状态转换更加清晰和可控。
  6. 防止重复点击

    • 可以考虑在点击事件处理中添加防抖逻辑,防止用户快速连续点击导致的问题。
  7. 优化属性绑定

    • 如果DDT.TraySortOrderModel.actionsAlwaysVisible变化不频繁,可以考虑使用onActionsAlwaysVisibleChanged信号来更新悬停状态,而不是持续绑定。

总结

这段代码整体质量较高,逻辑清晰,性能良好。主要的改进空间在于提高代码可读性和可维护性,以及增强错误处理能力。通过上述改进建议,可以使代码更加健壮和易于维护。

@yixinshark
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Sep 19, 2025

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit d963ead into linuxdeepin:master Sep 19, 2025
8 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants